-
-
Notifications
You must be signed in to change notification settings - Fork 482
refactor: Update Client.run
to have a better async I/O usage
#2645
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Co-authored-by: Dorukyum <[email protected]> Signed-off-by: DA344 <[email protected]>
Co-authored-by: Dorukyum <[email protected]> Signed-off-by: DA344 <[email protected]>
Applied all the changes Dorukyum requested. Before merging, I would like some feedback on this discussion message |
This has been fixed on the latest commit. |
I'll review and test that around 1pm |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still having issues. Seems like changing to _loop
in DiscordWebSocket
fixes it but I'll let you confirm.
INFO:discord.client:logging in using static token
Traceback (most recent call last):
File "C:\Users\jerem\Documents\pycord\pycord\test_client_async.py", line 39, in <module>
asyncio.run(main())
File "C:\Users\jerem\AppData\Roaming\uv\python\cpython-3.12.11-windows-x86_64-none\Lib\asyncio\runners.py", line 195, in run
return runner.run(main)
^^^^^^^^^^^^^^^^
File "C:\Users\jerem\AppData\Roaming\uv\python\cpython-3.12.11-windows-x86_64-none\Lib\asyncio\runners.py", line 118, in run
return self._loop.run_until_complete(task)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "C:\Users\jerem\AppData\Roaming\uv\python\cpython-3.12.11-windows-x86_64-none\Lib\asyncio\base_events.py", line 691, in run_until_complete
return future.result()
^^^^^^^^^^^^^^^
File "C:\Users\jerem\Documents\pycord\pycord\test_client_async.py", line 33, in main
await bot.start(os.getenv("TOKEN_2", ""))
File "C:\Users\jerem\Documents\pycord\pycord\discord\client.py", line 872, in start
await self.connect(reconnect=reconnect)
File "C:\Users\jerem\Documents\pycord\pycord\discord\client.py", line 759, in connect
self.ws = await asyncio.wait_for(coro, timeout=60.0)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "C:\Users\jerem\AppData\Roaming\uv\python\cpython-3.12.11-windows-x86_64-none\Lib\asyncio\tasks.py", line 520, in wait_for
return await fut
^^^^^^^^^
File "C:\Users\jerem\Documents\pycord\pycord\discord\gateway.py", line 348, in from_client
ws = cls(socket, loop=client.loop)
^^^^^^^^^^^
File "C:\Users\jerem\Documents\pycord\pycord\discord\client.py", line 367, in loop
raise RuntimeError("loop is not set")
RuntimeError: loop is not set
ERROR:asyncio:Unclosed client session
client_session: <aiohttp.client.ClientSession object at 0x0000019183896540>
ERROR:asyncio:Unclosed connector
connections: ['deque([(<aiohttp.client_proto.ResponseHandler object at 0x00000191843B9A30>, 47503.546)])']
connector: <aiohttp.connector.TCPConnector object at 0x0000019183A48F80>
Test Code
import os
from discord import *
import asyncio
import logging
from dotenv import load_dotenv
logging.basicConfig(level=logging.INFO)
load_dotenv()
bot = Bot(intents=Intents.all())
class MyCog(Cog):
"""Example cog for the bot"""
def __init__(self, bot: Bot):
self.bot: Bot = bot
@Cog.listener()
async def on_ready(self):
print(f"Logged in as {self.bot.user} (ID: {self.bot.user.id})")
async def load_cogs():
bot.add_cog(MyCog(bot))
async def main():
await load_cogs()
try:
await bot.start(os.getenv("TOKEN_2", ""))
except KeyboardInterrupt:
await bot.close()
if __name__ == "__main__":
asyncio.run(main())
Edit, no, that's not all, if you leave it running for some time after, leads to
Exception in thread Thread-1:
Traceback (most recent call last):
File "C:\Users\jerem\AppData\Roaming\uv\python\cpython-3.12.11-windows-x86_64-none\Lib\threading.py", line 1075, in _bootstrap_inner
self.run()
File "C:\Users\jerem\Documents\pycord\pycord\discord\gateway.py", line 167, in run
f = asyncio.run_coroutine_threadsafe(coro, loop=self.ws.loop)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "C:\Users\jerem\AppData\Roaming\uv\python\cpython-3.12.11-windows-x86_64-none\Lib\asyncio\tasks.py", line 950, in run_coroutine_threadsafe
loop.call_soon_threadsafe(callback)
^^^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'NoneType' object has no attribute 'call_soon_threadsafe'
C:\Users\jerem\AppData\Roaming\uv\python\cpython-3.12.11-windows-x86_64-none\Lib\threading.py:1077: RuntimeWarning: coroutine 'DiscordWebSocket.send_heartbeat' was never awaited
self._invoke_excepthook(self)
RuntimeWarning: Enable tracemalloc to get the object allocation traceback
WARNING:discord.gateway:Can't keep up, shard ID None websocket is 49.8s behind.
WARNING:discord.gateway:Can't keep up, shard ID None websocket is 99.4s behind.
WARNING:discord.gateway:Can't keep up, shard ID None websocket is 149.0s behind.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
test and works great with and without my overlap pr, no idea about paillat issue tho
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alright looks good, can't find any problem. Merge coflicts, and @Lumabots please test your loop stuff one last time with the latest changes.
tested and no issue with #2771 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@Lulalaby when your merge this (once review is good) pls make sure to squash so that we can put it in rc2 and then revert it easily if problems |
Signed-off-by: Paillat <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Luma comment
Summary
This PR refactors the
Client.run
logic to fix problems involving asyncio due to how the library used the loop:Needs testing
Exception
Information
examples, ...).
Checklist
type: ignore
comments were used, a comment is also left explaining why.